-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feat/#120] 게시물 떠먹기 API 연결 #123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨슴다~
…st-api # Conflicts: # app/src/main/java/com/spoony/spoony/data/datasource/PostRemoteDataSource.kt # app/src/main/java/com/spoony/spoony/data/datasourceimpl/PostRemoteDataSourceImpl.kt # app/src/main/java/com/spoony/spoony/data/dto/base/BaseResponse.kt # app/src/main/java/com/spoony/spoony/data/repositoryimpl/PostRepositoryImpl.kt # app/src/main/java/com/spoony/spoony/data/service/PostService.kt # app/src/main/java/com/spoony/spoony/presentation/placeDetail/PlaceDetailViewModel.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!! 명세서와 맞지 않은 부분이 있는 것 같은데 한번만 확인 부탁드립니다😊
@@ -1,11 +1,12 @@ | |||
package com.spoony.spoony.domain.repository | |||
|
|||
import com.spoony.spoony.data.dto.base.BaseResponse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: 잡았다!!!! Domain 레이어에서는 Data 레이어의 dto를 사용하면 안됩니다!! 이유가 무엇일까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data 계층과 domain 계층간의 결합도가 높아져서 입니다! 수정했습니다.
false -> { | ||
// 통신에 성공했지만 떠먹기에 실패했을 경우 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: 이 부분이 스푼이 부족해서 떠먹기에 실패한 경우가 맞을까요?? 명세서를 확인해보니 해당 상황에서는 요청이 반려된다고 해서 onSuccess가 아닌 onFailure로 처리해야 할 것 같습니다!! 이 부분 한번만 다시 확인 부탁드릴게요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정했습니다!
} | ||
} | ||
} | ||
.onFailure { | ||
// 실패 했을 경우 | ||
// 통신에 실패 했을 경우 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: 통신에 실패했을 경우 로직도 추가해주세요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
약간 걸리는 부분이 있어서 코리 남겨두었습니다..!! 한번만 확인 부탁드려요😊
_state.update { | ||
it.copy( | ||
postEntity = UiState.Success( | ||
copy(isScooped = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: 여기에 copy가 왜 또 들어가나요..???? 위에서 copy 해주고 있어서 안쪽에서는 안해도 될 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정했습니다!
with(currentPostEntity) { | ||
_state.update { | ||
it.copy( | ||
postEntity = UiState.Success( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: onFailure일 때도 UiState.Success로 넣어주고 있네요!! 제 생각에는 Failure로 해줘야 더 명확하지 않을까 싶습니다 세홍님 생각은 어떠세요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
얼렁 수정하자!!!!!
import okhttp3.internal.toImmutableList | ||
|
||
data class PostModel( | ||
val photoUrlList: List<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: ImmutableList로 수정해주세요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정했습니다!
|
||
import com.spoony.spoony.domain.entity.CategoryEntity | ||
import com.spoony.spoony.domain.entity.PostEntity | ||
import okhttp3.internal.toImmutableList |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: 임포트 잘못 들어간 것 같아요!! 다시 한번만 확인 부탁드립니다~~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이걸잡다니... 감사합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어푸푸!!🚀🚀
Related issue 🛠
Work Description ✏️
Screenshot 📸
To Reviewers 📢